feat(integrations): Add Elasticsearch integration#6793
Conversation
Instrument the elasticsearch-py sync and async clients through BaseClient.perform_request, which every API method funnels through. Creates db spans named after the API endpoint, adds query breadcrumbs, gates request bodies behind send_default_pii, and supports span streaming. Tests run against a real Elasticsearch node like the other DB suites, so the DBs CI group gains an elasticsearch service container.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 63524c4. Configure here.
| try: | ||
| yield | ||
| finally: | ||
| span.end() |
There was a problem hiding this comment.
Streamed spans miss error status
Medium Severity
When span streaming is enabled, _es_span ends the span with span.end() instead of exiting via the context manager. That path never sets SpanStatus.ERROR on exceptions, so failed Elasticsearch calls are recorded as successful spans.
Reviewed by Cursor Bugbot for commit 63524c4. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| try: | ||
| yield | ||
| finally: | ||
| span.end() |
There was a problem hiding this comment.
Bug: A failing Elasticsearch streaming operation does not mark its Sentry span as an error because span.end() is used instead of a with statement.
Severity: MEDIUM
Suggested Fix
Replace the try...finally block with a with sentry_sdk.start_span(...) as span: block for the streaming operation. This will ensure the span's __exit__ method is invoked with exception details, allowing it to correctly set the span status to 'error' upon failure.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry_sdk/integrations/elasticsearch.py#L132-L135
Potential issue: When an Elasticsearch operation fails in the streaming path, the
associated Sentry span's status remains 'ok' instead of being set to 'error'. This
occurs because the code uses a `try...finally` block where `span.end()` is called in the
`finally` clause. The `span.end()` method does not receive information about exceptions
from the `try` block. In contrast, the non-streaming path correctly uses a `with
sentry_sdk.start_span(...)` block, which ensures the span's `__exit__` method is called
with exception details, allowing the status to be set correctly on failure.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Description
Adds an
ElasticsearchIntegrationforelasticsearch-py, which #932 has been collecting requests for since 2020.Both the sync and the async client funnel every API call through their class's
perform_request, so the integration patches those two methods and covers the whole API surface with one seam. Each call gets adbspan plus aquerybreadcrumb.Design notes, happy to change any of these:
search,bulk,indices.refresh) using theendpoint_idthe client passes toperform_requestsince 8.12. On older clients the span name falls back toMETHOD /path. Attributes are an include-list:db.system,db.operation,server.address/server.port,url.path,http.request.method, anddb.elasticsearch.path_parts.*for the index/document id path parameters, following the OpenTelemetry Elasticsearch conventions.db.query.text) whensend_default_piiis enabled, since search bodies routinely contain user data. Same treatment as query params in the other DB integrations.server.address/server.portare only set when the client has exactly one configured node. With multiple nodes, only the transport layer knows which node actually served a request (it can also retry across nodes), so anything set at the client layer would be a guess.http.clientspans from the stdlib integration are left alone. That matches how the HTTP-based AI integrations behave and keeps transport retries visible.perform_requestsignature, which the test matrix verifies against 8.0.1.Tests run against a real Elasticsearch single node like the other DB suites, so this also adds an elasticsearch service container to the DBs test group and wires up
populate_tox/split_tox_gh_actions, with the generatedtox.iniand workflow changes included. For local runs:docker run -d -p 9200:9200 -e discovery.type=single-node -e xpack.security.enabled=false \ docker.elastic.co/elasticsearch/elasticsearch:9.1.4I'll follow up with the sentry-docs PR once this ships in a release.
Issues